authentication protocol for udp #2417

dshulyak · 2025-10-03T08:11:53Z

implementation of the #2091

dshulyak · 2025-10-03T08:12:45Z

this doesn't include integration code (e.g discovery, dataplane and raptorcast changes) and i still need to add more docs to the code itself

monad-wireauth-protocol/src/cookies.rs

dnkolegov-ar · 2025-10-09T15:19:43Z

monad-wireauth-protocol/src/common.rs

+    }
+}
+
+#[derive(Clone, Debug, Zeroize)]


There is no ZeroizeOnDrop. Is it intentional? It could be useful for tmp_x variables

yes, i am using Zeroizing for chaining_key and hash output. i think there was no need to zeroize every tmp variable, will recheck

my reasoning was that those values are always on the stack. so they will be overwritten shortly after function returns.
adding ZeroizeOnDrop overwrites them only a few ns earlier

it likely costs nothing to zeroize them, just feels weird.
boringtun doesn't zeroize intermidiate variables, i am curious how kernel implementation handles those.

dnkolegov-ar · 2025-10-09T15:30:51Z

monad-wireauth-protocol/src/handshake.rs

+            .expect("ephemeral private key must be set"),
+        &msg.ephemeral_public,
+    )?;
+    let temp = keyed_hash!(state.chaining_key.as_ref(), ecdh_ee.as_ref());


The ephemeral private key of the initiator's state can be erased here.

i guess i can .take() it from the state, but the state itself is dropped (and private key is erased at that point) soon after exiting this function

i kept it as is, dropping it slightly earlier should not make a difference

monad-wireauth-api/src/api.rs

monad-wireauth-session/src/common.rs

dnkolegov-ar · 2025-10-15T18:47:31Z

monad-wireauth-protocol/src/cookies.rs

+    )
+    .map_err(CookieError::CookieDecryptionFailed)?;
+
+    Ok(reply.encrypted_cookie)


Maybe it is worth adding a comment that this function decrypts the cookie and returns its value. No comments, and a returned reply.encrypted_cookie is misleading.

dnkolegov-ar · 2025-10-15T18:52:02Z

monad-wireauth-session/src/common.rs

+        Ok((terminated, rekey))
+    }
+
+    pub fn handle_rekey_timer(&mut self) -> RekeyEvent {


It is called handle_rekey_timer, but nothing is handled here.

thanks, i will refactor this

monad-wireauth-session/src/initiator.rs

dnkolegov-ar · 2025-10-15T19:12:13Z

monad-wireauth-api/src/cookie.rs

+
+use crate::error::{ProtocolErrorContext, Result};
+
+pub struct Cookies {


There is no zeroization for secrets

i don't think that it accomplishes anything. those secrets can be erased only when authentication protocol is terminated (e.g program closed for example).
and it doesn't matter if they are stolen when protocol is no longer working

monad-wireauth-api/src/cookie.rs

dnkolegov-ar · 2025-10-15T19:25:34Z

monad-wireauth-session/src/common.rs

+    pub ip_rate_limit_window: Duration,
+    pub max_requests_per_ip: usize,
+    pub ip_history_capacity: usize,
+    pub psk: [u8; 32],


Do we need to add zeroization for Config?

i think it makes sense to erase psk, as it meant to be long term secret

dnkolegov-ar · 2025-10-15T19:25:59Z

monad-wireauth-session/src/common.rs

+    pub rekey_deadline: Option<Duration>,
+    pub session_timeout_deadline: Option<Duration>,
+    pub max_session_duration_deadline: Option<Duration>,
+    pub stored_cookie: Option<[u8; 16]>,


Do we need to add zeroization for the cookie secret?

i think it only allows attacker to bypass dos-filter.
it doesn't cost much to wrap this in Zerozing, but practically it seemed to me unnecessary

dnkolegov-ar · 2025-10-16T08:39:39Z

monad-wireauth-api/src/api.rs

+use crate::{Initiator, Responder, Transport};
+use monad_wireauth_session::{Config, SessionIndex};
+
+pub struct API<C: Context> {


In WireGuard:

after an initiator receives a handshake response message (section 5.4.3), the responder cannot send transport data messages (section 5.4.6) until it has received the first transport data message from the initiator. And, further, transport data messages encrypted using the previous secure session might be in transit after a new secure session has been created. For these reasons, WireGuard keeps in memory the current secure session, the previous secure session, and the next secure session for the case of an unconfirmed session

Could you point me to the code where this logic is implemented? Or explain why this is not implemented and why the protocol does not need that functionality.

I think it should be in

monad-bft/monad-wireauth-api/src/api.rs

Line 400 in 98a6b9b

fn decrypt_data_packet(&mut self, packet: &mut [u8], remote_addr: SocketAddr) -> Result<bool> {

, but I do not see the entire workflow, only the mechanism that ensures that the initiator must send some data to the responder before the responder starts sending data to the initiator.

after an initiator receives a handshake response message (section 5.4.3), the responder cannot send transport data messages (section 5.4.6) until it has received the first transport data message from the initiator.

implemented by having a separate type for Responder, that type can't send messages, but can accept them (decrypt method), and on first successfully decrypted message it transitions to Transport type (establish method)

And, further, transport data messages encrypted using the previous secure session might be in transit after a new secure session has been created. For these reasons, WireGuard keeps in memory the current secure session, the previous secure session, and the next secure session for the case of an unconfirmed session

in api crate (state.rs) i track sessions with different state separately, sessions in Transport state can't be replaced by unconfirmed sessions, once Transport is created there is a call handle_established that evicts older sessions. that covers current secure session (Transport type) and separate unconfirmed session. so initiator or responder will never evict established session until new session transitions to Transport state

there is currently an expectation that both sides will initiate connect before sending messages, and i track 1 of each initiated and accepted instead of previous secure session. it addresses a problem that older session may get evicted during rekey while there are inflight messages

though this last part is not very intuitive, i will check how to transition to simply storing 2 last established sessions instead

monad-wireauth-api/src/state.rs

monad-wireauth-api/src/api.rs

monad-wireauth-protocol/src/crypto.rs

dnkolegov-ar · 2025-10-26T18:23:47Z

@dshulyak When can I review the fixes for the findings? I think it makes sense to do one more round of the security review after you fix all current findings/comments.

dshulyak · 2025-10-27T08:18:10Z

@dnkolegov-ar i will push changes by wed 29th

dshulyak · 2025-10-28T13:31:33Z

@dnkolegov-ar i pushed a separate commit with changes for comments

dshulyak force-pushed the dmitry/wireauth branch 3 times, most recently from 070fb7c to 98a6b9b Compare October 8, 2025 12:34

dnkolegov-ar reviewed Oct 9, 2025

View reviewed changes

monad-wireauth-protocol/src/cookies.rs Show resolved Hide resolved

dnkolegov-ar reviewed Oct 9, 2025

View reviewed changes

dnkolegov-ar reviewed Oct 14, 2025

View reviewed changes

monad-wireauth-api/src/api.rs Outdated Show resolved Hide resolved

dnkolegov-ar reviewed Oct 15, 2025

View reviewed changes

dnkolegov-ar reviewed Oct 16, 2025

View reviewed changes

monad-wireauth-api/src/state.rs Outdated Show resolved Hide resolved

monad-wireauth-api/src/state.rs Outdated Show resolved Hide resolved

dnkolegov-ar reviewed Oct 24, 2025

View reviewed changes

monad-wireauth-api/src/api.rs Outdated Show resolved Hide resolved

monad-wireauth-api/src/api.rs Show resolved Hide resolved

dnkolegov-ar reviewed Oct 24, 2025

View reviewed changes

monad-wireauth-api/src/api.rs Outdated Show resolved Hide resolved

monad-wireauth-protocol/src/crypto.rs Show resolved Hide resolved

dshulyak force-pushed the dmitry/wireauth branch from 98a6b9b to e96c43a Compare October 28, 2025 13:19

dshulyak added 2 commits October 28, 2025 14:39

authentication protocol for udp

9961e4e

address feedback

41d2351

dshulyak force-pushed the dmitry/wireauth branch from e96c43a to 41d2351 Compare October 28, 2025 13:41


		use crate::error::{ProtocolErrorContext, Result};

		pub struct Cookies {

authentication protocol for udp #2417

Are you sure you want to change the base?

authentication protocol for udp #2417

Conversation

dshulyak commented Oct 3, 2025

Uh oh!

dshulyak commented Oct 3, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnkolegov-ar Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnkolegov-ar commented Oct 26, 2025

Uh oh!

dshulyak commented Oct 27, 2025

Uh oh!

dshulyak commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dnkolegov-ar Oct 16, 2025 •

edited

Loading